Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable ed25519 to be built in wasm target with wasm32_c gate #1176

Closed
wants to merge 1 commit into from
Closed

Enable ed25519 to be built in wasm target with wasm32_c gate #1176

wants to merge 1 commit into from

Conversation

acmiyaguchi
Copy link

I was curious about #918 and tried enabling a few tests to understand what's going on here. The curve25519 module doesn't seems to depend on assembly functions, so it looks safe to enable as per #918 (comment).

Before enabling these tests, I verified that I could reproduce an error about missing a module named env.

% wasm-pack test --firefox --headless --  --test ed25519_tests 
...
[INFO]: ⬇️  Installing wasm-bindgen...
    Finished test [unoptimized + debuginfo] target(s) in 0.05s
     Running target/wasm32-unknown-unknown/debug/deps/ed25519_tests-af557e71142febbd.wasm
...
driver stderr:
    *** You are running in headless mode.
    JavaScript error: http://127.0.0.1:62601/wasm-bindgen-test, line 1: TypeError: Error resolving module specifier “env”. Relative module specifiers must start with “./”, “../” or “/”.
Full output 
% wasm-pack test --firefox --headless --  --test ed25519_tests 
[INFO]: 🎯  Checking for the Wasm target...
   Compiling ring v0.16.19 (/Users/amiyaguchi/Work/ring)
    Finished dev [unoptimized + debuginfo] target(s) in 1.86s
[INFO]: ⬇️  Installing wasm-bindgen...
    Finished test [unoptimized + debuginfo] target(s) in 0.05s
     Running target/wasm32-unknown-unknown/debug/deps/ed25519_tests-af557e71142febbd.wasm
Set timeout to 20 seconds...
Running headless tests in Firefox on `http://127.0.0.1:62602/`
Try find `webdriver.json` for configure browser's capabilities:
Not found
Failed to detect test as having been run. It might have timed out.
output div contained:
    Loading scripts...

driver status: signal: 9                          
driver stdout:
    1611356824151       geckodriver     INFO    Listening on 127.0.0.1:62602
    1611356824255       mozrunner::runner       INFO    Running command: "/Applications/Firefox.app/Contents/MacOS/firefox-bin" "--marionette" "-headless" "-foreground" "-no-remote" "-profile" "/var/folders/q6/m7bqqf2n05l5dvng72_xvvlw0000gn/T/rust_mozprofileZY7YL3"
    console.warn: SearchSettings: "get: No settings file exists, new profile?" (new Error("", "(unknown module)"))
    1611356825619       Marionette      INFO    Listening on port 62606
    1611356845909       Marionette      INFO    Stopped listening on port 62606

driver stderr:
    *** You are running in headless mode.
    JavaScript error: http://127.0.0.1:62601/wasm-bindgen-test, line 1: TypeError: Error resolving module specifier “env”. Relative module specifiers must start with “./”, “../” or “/”.

Error: some tests failed                          
error: test failed, to rerun pass '--test ed25519_tests'
Error: Running Wasm tests with wasm-bindgen-test failed
Caused by: failed to execute `cargo test`: exited with exit code: 1
  full command: "cargo" "test" "--target" "wasm32-unknown-unknown" "--test" "ed25519_tests"

The wat output shows that the extern'ed C functions are not available to wasm.

% wasm2wat target/wasm32-unknown-unknown/debug/deps/ed25519_tests-af557e71142febbd.wasm | grep '(import "env"' 
  (import "env" "GFp_x25519_ge_scalarmult_base" (func $GFp_x25519_ge_scalarmult_base (type 5)))
  (import "env" "GFp_x25519_sc_muladd" (func $GFp_x25519_sc_muladd (type 4)))
  (import "env" "GFp_x25519_sc_reduce" (func $GFp_x25519_sc_reduce (type 3)))
  (import "env" "GFp_x25519_sc_mask" (func $GFp_x25519_sc_mask (type 3)))
  (import "env" "GFp_x25519_fe_neg" (func $GFp_x25519_fe_neg (type 3)))
  (import "env" "GFp_x25519_ge_frombytes_vartime" (func $GFp_x25519_ge_frombytes_vartime (type 11)))
  (import "env" "GFp_x25519_fe_invert" (func $GFp_x25519_fe_invert (type 5)))
  (import "env" "GFp_x25519_fe_mul_ttt" (func $GFp_x25519_fe_mul_ttt (type 6)))
  (import "env" "GFp_x25519_fe_tobytes" (func $GFp_x25519_fe_tobytes (type 5)))
  (import "env" "GFp_x25519_fe_isnegative" (func $GFp_x25519_fe_isnegative (type 18)))
  (import "env" "GFp_x25519_ge_double_scalarmult_vartime" (func $GFp_x25519_ge_double_scalarmult_vartime (type 4)))
  (import "env" "LIMBS_less_than" (func $LIMBS_less_than (type 12)))
  (import "env" "LIMBS_are_zero" (func $LIMBS_are_zero (type 11)))

This patch enables the tests and allows the appropriate C file to be compiled when the wasm32_c feature is enabled. This was tested using the following command.

 % wasm-pack test --firefox --headless --  --features wasm32_c --test ed25519_tests

Is there a place that would be appropriate for documenting how to run the wasm-bindgen tests? It's not obvious unless you're already familiar with the workflow.

@codecov
Copy link

codecov bot commented Jan 23, 2021 •
edited
Loading

Codecov Report

Merging #1176 (0becf93) into main (c263876) will decrease coverage by 1.35%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1176      +/-   ##
==========================================
- Coverage   93.63%   92.27%   -1.36%     
==========================================
  Files         116      116              
  Lines       17397    17655     +258     
==========================================
+ Hits        16289    16291       +2     
- Misses       1108     1364     +256
Impacted Files Coverage Δ
tests/ed25519_tests.rs 93.75% <ø> (ø)
src/aead/unbound_key.rs 50.00% <0.00%> (-30.00%) ⬇️
src/hkdf.rs 72.64% <0.00%> (-20.13%) ⬇️
src/aead/chacha20_poly1305_openssh.rs 88.09% <0.00%> (-11.91%) ⬇️
src/ec/curve25519/ed25519/verification.rs 85.29% <0.00%> (-8.26%) ⬇️
src/error.rs 86.00% <0.00%> (-5.49%) ⬇️
src/ec/curve25519/x25519.rs 94.31% <0.00%> (-4.50%) ⬇️
tests/agreement_tests.rs 93.00% <0.00%> (-4.08%) ⬇️
src/test.rs 87.95% <0.00%> (-1.08%) ⬇️
src/aead/less_safe_key.rs 95.61% <0.00%> (-0.88%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c263876...0becf93. Read the comment docs.

@briansmith
Copy link
Owner

Do you need the signing code, or only the verification code? It's safer/easier for me to enable the verification code since I don't have to worry about the timing side channels.

@acmiyaguchi
Copy link
Author

I'm actually interested in using the key agreement module (x25519) in a hybrid encryption scheme. My understanding of cryptography is a bit weak, so I assume side-channel attacks are still relevant in that context too?

@briansmith
Copy link
Owner

Due to no fault of your own, this PR is out of date with the current way we're handling tests in wasm32 browser environments. I made PR #1440 to get Ed25519 working in wasm32, including the tests, using the current "right" way of doing it.

@briansmith
Copy link
Owner

I'm actually interested in using the key agreement module (x25519) in a hybrid encryption scheme. My understanding of cryptography is a bit weak, so I assume side-channel attacks are still relevant in that context too?

I realize I'm replying to an old message. If you are still interested in X25519, it can be made to work in wasm32 as well (as can everything else that's not currently enabled for wasm32), but PR #1440 is focused on Ed25519.

Since this PR also only focuses on Ed25519, I'm going to close it.

@briansmith briansmith closed this Dec 16, 2021
@briansmith
Copy link
Owner

Also, thanks for contributing this. Sorry I overlooked it at the time.

@paulyoung paulyoung mentioned this pull request Oct 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@acmiyaguchi @briansmith